Revert "feat(disability_registry): stabilization improvements for spp_disability_registry (#1047)" (#263)#268
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the spp_disability_registry module by removing the child functioning module (CFM) questionnaire fields, the configuration settings, and the separate impairment and device request models, refactoring them into direct fields on the assessment model. It also removes the spp_starter_disability_registry starter bundle. Feedback on these changes highlights two critical issues: first, using @api.onchange to set is_proxy_response fails to trigger during programmatic creation, so a stored computed field should be used instead; second, removing the readonly attribute from the assessment form view allows approved or pending records to be edited, and the dynamic read-only state should be restored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @api.onchange("assessment_type") | ||
| def _onchange_assessment_type(self): | ||
| """Set proxy response flag automatically for child assessments.""" | ||
| if self.assessment_type in ("cfm_2_4", "cfm_5_17"): | ||
| self.is_proxy_response = True |
There was a problem hiding this comment.
Using @api.onchange to set is_proxy_response means this logic will only execute in the web client interface. When assessments are created programmatically (e.g., via create(), API, or demo data), the onchange is not triggered, leaving is_proxy_response as False for child assessments. This is also evident in the test test_proxy_flag_set_for_child where _onchange_assessment_type() has to be called manually.
To ensure this business rule is always enforced, is_proxy_response should be a computed field (with store=True and readonly=False if manual overrides are allowed).
| string="Disability Assessment" | ||
| readonly="approval_state not in ('draft', 'revision')" | ||
| > | ||
| <form string="Disability Assessment"> |
There was a problem hiding this comment.
Removing the readonly attribute from the <form> tag makes the entire assessment form editable even after the assessment has been approved or is pending validation. Approved disability records should be read-only to prevent unauthorized modifications.
Please restore the dynamic read-only state on the form view using the conventional Odoo 17+ pattern:
<form string="Disability Assessment" readonly="approval_state not in ('draft', 'revision')">References
- In Odoo 17+, use to dynamically set the read-only state for all fields in a form view, as this is the supported and conventional pattern.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #268 +/- ##
==========================================
- Coverage 75.31% 75.30% -0.02%
==========================================
Files 1098 1096 -2
Lines 65317 64992 -325
==========================================
- Hits 49191 48939 -252
+ Misses 16126 16053 -73
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
1 participant
Why is this change needed?
Reverts the merge of PR #263 (merge commit
f260ffd3) from19.0. The Disability Registry stabilization work needs to be pulled back out of19.0for now and re-reviewed before it lands.How was the change implemented?
git revert -m 1 f260ffd3— reverts the merge against mainline (parent 1), removing allspp_disability_registry/spp_starter_disability_registrychanges introduced by #263. No history rewrite.Note
To re-introduce this work later it must be reapplied (revert-of-this-revert or a rebased branch) — a fresh PR from the original branch tip alone will show an empty diff because those commits remain ancestors of
19.0. A draft PR reapplying the work will be opened separately.Related links
Reverts #263 · OP#1047